Skip to content

fix(sandboxes): align CLI with hosting-2 lifetime contract - #294

Merged
shawnwu-kong merged 2 commits into
mainfrom
fix/sandbox-cli-hosting-2
Oct 9, 2026
Merged

shawnwu-kong merged 2 commits into
mainfrom
fix/sandbox-cli-hosting-2

Conversation

@shawnwu-kong

@shawnwu-kong shawnwu-kong commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Fix Sandbox configuration deploys returning HTTP 400 after Hosting #1743 removed idle_timeout_seconds. Remove the retired manifest field and round-trip expectations, reject it locally, and document ttl_seconds including local unlimited lifetime. Refresh the vendored OpenAPI contract and generated client from Hosting 729519c2d, including nullable Sandbox session fields.

sandboxes run now omits max_duration_seconds unless --duration is supplied, so sessions inherit template lifetimes. Explicit durations accept zero (unlimited) or positive values locally and 30–28800 seconds in cloud mode. This fixes the P2 where an unconditional 3600-second duration overrode unlimited templates.

Validation: projectconfig, config commands, generated client, API, Sandbox commands, manifest extraction, and documentation tests pass. Scoped lint, formatting, and generated-code consistency pass. The lifetime regression test reproduces the prior failure and passes after the fix across local/cloud modes, preset/template selectors, and duration boundaries. Local-mode E2E against Hosting 729519c2d (kong/volcano:hosting2-cli-check) passes, including config dry-run/deploy/pull, inherited 600-second expiry, explicit unlimited override, and inherited unlimited lifetime. Remote CI is rerunning for the latest commit.

@shawnwu-kong
shawnwu-kong requested a review from a team as a code owner October 9, 2026 21:55
@shawnwu-kong

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T21:59:40.713752Z 3d01df5 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d01df56e5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +394 to +395
cloud project. Omitted fields preserve current settings. Cloud TTL must be
30–28800 seconds. Local mode accepts `0` for unlimited lifetime or a positive TTL.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor unlimited template lifetimes in local sessions

When a local template is configured with the newly supported ttl_seconds: 0, volcano sandboxes run --template ... still initializes --duration to 3600 and always sends it as MaxDurationSeconds (internal/cmd/sandboxes/exec.go:155-177), overriding the template default; explicitly passing --duration 0 is also rejected. Thus the advertised unlimited lifetime cannot be used by the CLI's session-creation command. Leave the duration omitted unless the flag was supplied, and validate zero according to local/cloud mode.

AGENTS.md reference: AGENTS.md:L9-L10

Useful? React with 👍 / 👎.

@shawnwu-kong
shawnwu-kong enabled auto-merge October 9, 2026 22:04
@shawnwu-kong
shawnwu-kong disabled auto-merge October 9, 2026 22:27
@shawnwu-kong
shawnwu-kong merged commit 3d5b7f8 into main Oct 9, 2026
10 of 11 checks passed
@shawnwu-kong
shawnwu-kong deleted the fix/sandbox-cli-hosting-2 branch October 9, 2026 22:27
@kong-volcano-app kong-volcano-app Bot mentioned this pull request Oct 9, 2026
@marckong

Copy link
Copy Markdown
Contributor

Post-merge review of this change (approve, with notes). It fixes the local-mode HTTP 400 after Hosting #1743 removed idle_timeout_seconds, and the --duration change is sound: omitted means inherit, and the cloud and local ranges are validated and covered by the matrix test.

  • [P2] config pull writes the server's manifest YAML as returned, and pull_sanitize.go only strips variables. Against a Hosting server without #1743, the pulled file still contains idle_timeout_seconds, and the next config deploy fails with field idle_timeout_seconds not found. Hosting #1743 needs to be deployed wherever the pending 0.43.4 release (release: 0.44.0 #295) is used, or the pull path could drop the field.
  • [P3] The rejection surfaces as a raw YAML "field not found" error. The docs say to remove the field, but the CLI error does not.
  • [P3] Hosting PRs that predate #1743 will now fail local-mode E2E, because the new test sets ttl_seconds: 0 and expects no expiry, and an older server rejects TTL below 30. Those PRs need a rebase.

I did not review the generated client or OpenAPI refresh line by line, and did not run the CLI's CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants